staticaddr/deposit: preserve confirmation heights - #1219
Conversation
053004f to
9c14505
Compare
|
/gateway review |
|
👀 gateway review starting… |
|
/gateway re-review |
|
gateway is not active on this PR. Run |
|
/gateway review |
starius
left a comment
There was a problem hiding this comment.
LGTM! 🎉
Everything is good. Added two nits.
| select { | ||
| case <-ticker.C: | ||
| err := m.reconcileDeposits(ctx) | ||
| _, err := m.reconcileDeposits(ctx) |
There was a problem hiding this comment.
Non-blocking: if a block-driven reconciliation returns no stable snapshot, that block's expiry notification is skipped. When this polling path later obtains a stable snapshot, it discards the reconciled tip and does not retry expiry processing, so an already-expired deposit waits until the next block. That is safe and conservative, but could delay a sweep. Would it be worth adding a test for the unsynced → synced recovery path, and either documenting the next-block delay or replaying the latest authoritative height once?
Bracket wallet UTXO queries with matching chain-synced lnd tips before converting confirmation counts into absolute heights. Defer expiry and spending decisions while the wallet is catching up, and cap queued expiry notifications at the reconciled tip.
9c14505 to
b0d0e4c
Compare
Static Address deposit reconciliation previously combined queued block epochs with wallet confirmation counts from a different lnd sync point. During rapid lnd catch-up this repeatedly rewrote the persisted first-confirmation height and could make expiry appear eligible one block early.
This change brackets each wallet UTXO query with matching, chain-synced lnd tip observations before deriving absolute confirmation heights. It also:
Stack order (bottom to top / merge order):
9c145058) — confirmation-height preservation (this PR), based onmaster.c74ac6cd) — multi-address core, stacked on this PR.8ab455f9) — withdrawal hardening, stacked on staticaddr: support multiple receive and change addresses #1218.Testing: